Abort after `representability` errors by nnethercote · Pull Request #153317 · rust-lang/rust

nnethercote · 2026-03-03T00:03:50Z

Doing so results in better error messages and makes the code a bit simpler. Details in individual commits.

rustbot · 2026-03-03T00:03:54Z

oli-obk is not on the review rotation at the moment.
They may take a while to respond.

nnethercote · 2026-03-03T00:11:10Z

This overlaps with #153028 by @Zoxc. (I wrote this independently a few days ago after writing #153169 and then subsequently discovered the overlap.) That PR changes the representability queries and also adds per-query cycle handlers. Those changes are all combined into a single large commit and the review comments have been mixed, so I thought it worth creating this PR for just the representability changes. They improve error messages, make the code simpler, don't get in the way of removing query cycle handling, but also don't commit to removing query cycle handling (which @zetanumbers has concerns about). I have also introduced the use of ensure_ok() and added check_ prefixes to the queries, which I think are good.

nnethercote · 2026-03-03T01:48:18Z

I have also introduced the use of ensure_ok()

Turns out this doesn't work for the reason because it requires dropping anon from the queries, which caused an incremental test failure. So I have undone that part, and I am just using let _ = tcx.check_representability(...);.

zetanumbers · 2026-03-03T06:39:45Z

compiler/rustc_middle/src/ty/adt.rs

-    Representable,
-    Infinite(ErrorGuaranteed),
-}
+pub struct Representability;


I like that you haven't got rid of this type, although I would have made it pub struct Representability(()) in order to make its construction a private detail for a query implementation.

Hmm, but we cannot do that as query implementation is in another crate. I guess I would've done new_unchecked unsafe fn instead then if rustc was my project, but that's kinda a far too theoretical concern for an open project.

petrochenkov · 2026-03-03T10:22:38Z

I have also introduced the use of ensure_ok()

Turns out this doesn't work for the reason because it requires dropping anon from the queries, which caused an incremental test failure. So I have undone that part, and I am just using let _ = tcx.check_representability(...);.

The second commit message still mentions ensure_ok and anon

Currently, `Representability::from_cycle_error` prints an "infinite size" error and then returns `Representability::Infinite`, which lets analysis continue. This commit changes it so it just aborts after printing the error. This has two benefits. First, the error messages are better. The error messages we get after continuing are mostly bad -- we usually get another cycle error, e.g. about drop checking or layout, which is not much use to the user, and then abort after that. The only exception is `issue-105231.rs` where a "conflicting implementations" error is now omitted, but there are three other errors before that one so it's no great loss. Second, it allows some simplifications: see the next commit.

This variant was a fallback value used to continue analysis after a `Representability` error, but it's no longer needed thanks to the previous commit. This means `Representability` can now be reduced to a unit type that exists just so `FromCycleError` can exist for the `representability` query. (There is potential for additional cleanups here, but those are beyond the scope of this PR.) This means the `representability`/`representability_adt_ty` queries no longer have a meaningful return value, i.e. they are check-only queries. So they now have a `check_` prefix added. And the `rtry!` macro is no longer needed.

rustbot · 2026-03-03T20:15:06Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

nnethercote · 2026-03-03T20:15:33Z

I have rebased and removed the anon mention from the second commit message.

oli-obk · 2026-03-04T06:57:34Z

compiler/rustc_middle/src/ty/adt.rs

-    Representable,
-    Infinite(ErrorGuaranteed),
-}
+pub struct Representability;


if you want to keep enforcing let _ = in front of the query invocations, then this struct should be #[must_use]. Otherwise just call the query like we call unit queries

If I change this:

let _ = tcx.check_representability(item.owner_id.def_id);

to this:

tcx.check_representability(item.owner_id.def_id);

I get this warning:

warning: unused return value of `queries::<impl rustc_middle::ty::TyCtxt<'tcx>>::check_representability` that must be used --> compiler/rustc_hir_analysis/src/check/wfcheck.rs:1000:5 | 1000 | tcx.check_representability(item.owner_id.def_id); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `#[warn(unused_must_use)]` (part of `#[warn(unused)]`) on by default help: use `let _ = ...` to ignore the resulting value | 1000 | let _ = tcx.check_representability(item.owner_id.def_id); | +++++++

I'm not sure where the must_use that triggers this comes from.

I could instead do this:

tcx.ensure_ok().check_representability(item.owner_id.def_id);

which is probably better, given that ensure_ok can help with incremental perf. Sound ok to you?

Oh, I totally goldfished that I tried ensure_ok before.

Anyway, I don't see how to avoid the let _ = things, given that I get the warnings without them.

Right. Unfortunate but makes sense for now

oli-obk · 2026-03-04T10:39:44Z

@bors r+

rust-bors · 2026-03-04T10:39:48Z

📌 Commit ff3d308 has been approved by oli-obk

It is now in the queue for this repository.

@oli-obk

…rrors, r=oli-obk Abort after `representability` errors Doing so results in better error messages and makes the code a bit simpler. Details in individual commits. r? @oli-obk

@oli-obk

…rrors, r=oli-obk Abort after `representability` errors Doing so results in better error messages and makes the code a bit simpler. Details in individual commits. r? @oli-obk

@oli-obk

…rrors, r=oli-obk Abort after `representability` errors Doing so results in better error messages and makes the code a bit simpler. Details in individual commits. r? @oli-obk

@oli-obk

…rrors, r=oli-obk Abort after `representability` errors Doing so results in better error messages and makes the code a bit simpler. Details in individual commits. r? @oli-obk

Rollup of 9 pull requests Successful merges: - #152164 (Lint unused features) - #152801 (Refactor WriteBackendMethods a bit) - #153317 (Abort after `representability` errors) - #153361 (enable `PassMode::Indirect { on_stack: true, .. }` tail call arguments) - #153402 (miri subtree update) - #153276 (Remove `cycle_fatal` query modifier) - #153396 (use `minicore` in some `run-make` tests) - #153401 (Migrationg of `LintDiagnostic` - part 7) - #153406 (Remove a ping for myself)

…uwer Rollup of 12 pull requests Successful merges: - #153402 (miri subtree update) - #152164 (Lint unused features) - #152801 (Refactor WriteBackendMethods a bit) - #153196 (Update path separators to be available in const context) - #153204 (Add `#[must_use]` attribute to `HashMap` and `HashSet` constructors) - #153317 (Abort after `representability` errors) - #153276 (Remove `cycle_fatal` query modifier) - #153300 (Tweak some of our internal `#[rustc_*]` TEST attributes) - #153396 (use `minicore` in some `run-make` tests) - #153401 (Migrationg of `LintDiagnostic` - part 7) - #153406 (Remove a ping for myself) - #153414 (Rename translation -> formatting)

@oli-obk

Rollup merge of #153317 - nnethercote:abort-after-infinite-errors, r=oli-obk Abort after `representability` errors Doing so results in better error messages and makes the code a bit simpler. Details in individual commits. r? @oli-obk

…uwer Rollup of 12 pull requests Successful merges: - rust-lang/rust#153402 (miri subtree update) - rust-lang/rust#152164 (Lint unused features) - rust-lang/rust#152801 (Refactor WriteBackendMethods a bit) - rust-lang/rust#153196 (Update path separators to be available in const context) - rust-lang/rust#153204 (Add `#[must_use]` attribute to `HashMap` and `HashSet` constructors) - rust-lang/rust#153317 (Abort after `representability` errors) - rust-lang/rust#153276 (Remove `cycle_fatal` query modifier) - rust-lang/rust#153300 (Tweak some of our internal `#[rustc_*]` TEST attributes) - rust-lang/rust#153396 (use `minicore` in some `run-make` tests) - rust-lang/rust#153401 (Migrationg of `LintDiagnostic` - part 7) - rust-lang/rust#153406 (Remove a ping for myself) - rust-lang/rust#153414 (Rename translation -> formatting)

rustbot assigned oli-obk Mar 3, 2026

This comment has been minimized.

Sign in to view

nnethercote force-pushed the abort-after-infinite-errors branch from 44bc328 to 8675222 Compare March 3, 2026 01:46

zetanumbers reviewed Mar 3, 2026

View reviewed changes

zetanumbers mentioned this pull request Mar 3, 2026

Add per query cycle handlers and use them for representability #153028

Open

nnethercote added 2 commits March 4, 2026 07:02

nnethercote force-pushed the abort-after-infinite-errors branch from 8675222 to ff3d308 Compare March 3, 2026 20:15

oli-obk reviewed Mar 4, 2026

View reviewed changes

rust-bors bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 4, 2026

JonathanBrouwer mentioned this pull request Mar 4, 2026

Rollup of 6 pull requests #153403

Closed

JonathanBrouwer mentioned this pull request Mar 4, 2026

Rollup of 7 pull requests #153404

Closed

JonathanBrouwer mentioned this pull request Mar 4, 2026

Rollup of 6 pull requests #153405

Closed

GuillaumeGomez mentioned this pull request Mar 4, 2026

Rollup of 9 pull requests #153412

Closed

JonathanBrouwer mentioned this pull request Mar 4, 2026

Rollup of 12 pull requests #153416

Merged

rust-bors bot merged commit febe0bc into rust-lang:main Mar 5, 2026
11 checks passed

rustbot added this to the 1.96.0 milestone Mar 5, 2026

nnethercote deleted the abort-after-infinite-errors branch March 5, 2026 04:39

Uh oh!

Conversation

nnethercote commented Mar 3, 2026

Uh oh!

rustbot commented Mar 3, 2026

Uh oh!

nnethercote commented Mar 3, 2026

Uh oh!

This comment has been minimized.

nnethercote commented Mar 3, 2026

Uh oh!

zetanumbers Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

zetanumbers Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

petrochenkov commented Mar 3, 2026

Uh oh!

rustbot commented Mar 3, 2026

Uh oh!

nnethercote commented Mar 3, 2026

Uh oh!

oli-obk Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

nnethercote Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

nnethercote Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

oli-obk Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

oli-obk commented Mar 4, 2026

Uh oh!

rust-bors bot commented Mar 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants